Skip to content

Be more specific on ports for ICMP#496

Merged
manuelbuil merged 2 commits into
rancher:mainfrom
manuelbuil:icmp
May 29, 2026
Merged

Be more specific on ports for ICMP#496
manuelbuil merged 2 commits into
rancher:mainfrom
manuelbuil:icmp

Conversation

@manuelbuil

@manuelbuil manuelbuil commented Mar 18, 2026

Copy link
Copy Markdown
Contributor

A colleague got confused when configuring the AWS Security groups because 0/8 ports were not possible. I can see how the current docs could confuse users who are not familiar with the networking stack. This PR clarifies that ICMP has no port:

image

@manuelbuil manuelbuil requested a review from a team as a code owner March 18, 2026 12:05
@brandond

brandond commented Mar 18, 2026

Copy link
Copy Markdown
Member

I don't think it's any more correct to put the ICMP types in the "protocol" field than it is to put them in the "port" field. I know that the EC2 security group editor hides the numeric types and makes you pick from a list of human-readable type names... but they're not a protocol either. Someone who knows little enough about this to be confused by their presence in the Port field seems unlikely to be better served by conflating them with the Protocol, especially since we list only the numbers.

Maybe change the "Port" header to "Port or Type", or just add a note above the table noting that it lists ICMP types in the port column, with a link to the IANA type registry at https://www.iana.org/assignments/icmp-parameters/icmp-parameters.xhtml#icmp-parameters-types in case someone needs the names?

Signed-off-by: Manuel Buil <mbuil@suse.com>
@manuelbuil

Copy link
Copy Markdown
Contributor Author

I don't think it's any more correct to put the ICMP types in the "protocol" field than it is to put them in the "port" field. I know that the EC2 security group editor hides the numeric types and makes you pick from a list of human-readable type names... but they're not a protocol either. Someone who knows little enough about this to be confused by their presence in the Port field seems unlikely to be better served by conflating them with the Protocol, especially since we list only the numbers.

Maybe change the "Port" header to "Port or Type", or just add a note above the table noting that it lists ICMP types in the port column, with a link to the IANA type registry at https://www.iana.org/assignments/icmp-parameters/icmp-parameters.xhtml#icmp-parameters-types in case someone needs the names?

I tried with a small footnote

@brandond brandond left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

request for clarification

Comment thread docs/install/requirements.md Outdated
| 8472 | UDP | All RKE2 nodes | All RKE2 nodes | Cilium CNI with VXLAN
| 51871 | UDP | All RKE2 nodes | All RKE2 nodes | Cilium CNI with WireGuard

\* 8/0 is not a port but the [ICMP type](https://www.iana.org/assignments/icmp-parameters/icmp-parameters.xhtml#icmp-parameters-types). It is required for the network utility ping

@brandond brandond May 27, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is suggesting that you need types 8 and 0, correct? 8 is Echo, 0 is Echo Reply. Or is this type 8, code 0?

Suggested change
\* 8/0 is not a port but the [ICMP type](https://www.iana.org/assignments/icmp-parameters/icmp-parameters.xhtml#icmp-parameters-types). It is required for the network utility ping
\* 0 and 8 are not ports but [ICMP types](https://www.iana.org/assignments/icmp-parameters/icmp-parameters.xhtml#icmp-parameters-types). Cilium uses echo (ping) between nodes for health checks.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes echo and echo reply. ICMP Type 8 and ICMP Type 0

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, lets change this. I would read 8/0 as type 8 code 0, if this is meant to be ready as "0 and 8" then we should say that.

Comment thread docs/install/requirements.md Outdated
Co-authored-by: Brad Davidson <brad@oatmail.org>
@manuelbuil manuelbuil merged commit 2439897 into rancher:main May 29, 2026
3 checks passed
@manuelbuil manuelbuil deleted the icmp branch May 29, 2026 10:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants